typescript/cli/src/context/strategies/chain/chainResolver.ts (1)

141-147: Add a clearer guard for missing/legacy strategy config.

Right now a mis-shaped strategyConfig (old format or missing) will likely blow up inside getStrategyChainNames and the CLI error won’t be very friendly. Consider a quick guard so the user gets a clear message.

💡 Suggested tweak

-  const chains = getStrategyChainNames(rebalancerConfig.strategyConfig);
+  assert(
+    Array.isArray(rebalancerConfig.strategyConfig),
+    'Invalid rebalancer config: "strategyConfig" must be an array of strategies',
+  );
+  const chains = getStrategyChainNames(rebalancerConfig.strategyConfig);

typescript/rebalancer/src/core/WithSemaphore.test.ts (1)

20-25: Trim the as any casts in test routes/config.
These double-casts can hide API drift. Prefer typed fixtures (or satisfies RebalanceRoute) and a typed config builder so tests fail when the contract changes.

As per coding guidelines: Avoid unnecessary type casts (as assertions), especially as unknown as X double-casts in TypeScript.

Also applies to: 50-55, 72-79, 82-86, 99-104

typescript/rebalancer/src/tracking/InflightContextAdapter.test.ts (1)

16-28: Prefer typed stubs/fixtures over as any.
Consider using satisfies IActionTracker/satisfies MultiProvider with stubbed methods, and typed address constants for sender/recipient. It keeps the tests honest without losing Sinon flexibility.

As per coding guidelines: Use proper typing over any or type assertions in TypeScript.

Also applies to: 57-59, 119-121, 171-173, 183-185

typescript/rebalancer/src/core/WithInflightGuard.test.ts (1)

51-53: Use buildTestRebalanceRoute() instead of as any for RebalanceRoute objects. The test helper already exists and is imported in this file—using it with typed overrides (e.g., buildTestRebalanceRoute({ origin: 'ethereum', intentId: 'test-1' })) ensures the route shape matches the type and catches any breaking changes to the contract.

Lines affected: 51-53, 82-84, 112-114

Replace:

{ origin: 'ethereum', intentId: 'test-1' } as any

With:

buildTestRebalanceRoute({ origin: 'ethereum', intentId: 'test-1' })

.changeset/inflight-aware-base-strategy.md (1)

5-5: Changeset description should use past tense, not present.

The coding guidelines say to write these descriptions in past tense. Right now we've got "is extended", "extended", "updated" - those need a wee bit of adjustment to sound like they already happened, not like they're happening now.

✏️ Suggested fix

-BaseStrategy is extended with inflight-aware rebalancing capabilities and bridge configuration support. RebalancingRoute extended with optional bridge field for bridge selection. Added three protected methods: reserveCollateral() to prevent draining collateral needed for incoming transfers, simulatePendingRebalances() for optional balance simulation, and filterRebalances() to filter routes based on actual balance sufficiency. The getRebalancingRoutes() method updated to accept optional inflightContext and integrate the new methods. getCategorizedBalances() signature updated to accept optional pendingRebalances parameter. BaseStrategy, WeightedStrategy, and MinAmountStrategy constructors extended with optional bridges parameter (ChainMap<Address[]>) to store configured bridge addresses per chain.
+Extended BaseStrategy with inflight-aware rebalancing capabilities and bridge configuration support. Extended RebalancingRoute with optional bridge field for bridge selection. Added three protected methods: reserveCollateral() to prevent draining collateral needed for incoming transfers, simulatePendingRebalances() for optional balance simulation, and filterRebalances() to filter routes based on actual balance sufficiency. Updated the getRebalancingRoutes() method to accept optional inflightContext and integrate the new methods. Updated getCategorizedBalances() signature to accept optional pendingRebalances parameter. Extended BaseStrategy, WeightedStrategy, and MinAmountStrategy constructors with optional bridges parameter (ChainMap<Address[]>) to store configured bridge addresses per chain.

As per coding guidelines: "Write changeset descriptions in past tense describing what changed, not what will change."

typescript/rebalancer/src/utils/ExplorerClient.ts (3)

47-50: The normalizeHex function returns falsy values as-is, which could cause type issues.

When hex is null or undefined, this returns that value directly, but the return type is implicitly string. Not a swamp-sized problem since the caller handles the fields, but something to keep an eye on.

const normalizeHex = (hex: string): string => {
  if (!hex) return hex; // returns undefined/null but typed as string
  ...
};

---

246-259: Consider extracting the duplicated error handling into a helper method.

This error-response parsing logic appears three times in the file (here, lines 134-148, and 342-355). Could be nice and tidy with a wee private helper like parseErrorResponse(res: Response).

---

126-130: Fetch calls have no timeout configured.

These Explorer API requests could hang if the service is slow or unresponsive. Might want to consider adding an AbortController with a timeout to keep things from getting stuck in the swamp. Same applies to the other two fetch calls in this file.

typescript/rebalancer/src/strategy/StrategyFactory.test.ts (1)

99-106: Worth adding a test for CompositeStrategy creation in the factory tests.

Like an ogre with layers, this factory's got 'em—single strategy goes straight through, but multiple strategies wrap up in a CompositeStrategy. The tests here only check the single-strategy path, so adding one more test that passes a multi-element array would round out the coverage nicely. Not urgent, but it'd be good to have.

typescript/rebalancer/src/interfaces/IStrategy.ts (1)

16-25: Bridge the gap with a better type definition.
The filterByConfiguredBridges method casts to StrategyRoute just to grab the bridge property—feels like working around the type system. Since pendingRebalances sometimes has a bridge (like the comment says), might as well type it properly. The tests already do this same pattern, so it's a well-trodden path.

♻️ Type adjustment

+export type PendingRebalanceRoute = Route & { bridge?: Address };
+
 export type InflightContext = {
   /**
    * In-flight rebalances from ActionTracker.
    * Uses Route[] because recovered intents (from Explorer startup recovery)
    * don't have bridge information. Some routes may have bridge at runtime.
    */
-  pendingRebalances: Route[];
+  pendingRebalances: PendingRebalanceRoute[];
   pendingTransfers: Route[];
   /** Routes from earlier strategies - always have bridge */
   proposedRebalances?: StrategyRoute[];
 };

typescript/rebalancer/src/tracking/store/InMemoryStore.ts (1)

47-51: The as any cast here is like onion layers - it hides what's underneath.

The coding guidelines say to avoid unnecessary type casts and prefer type guards for runtime narrowing. Since Status is a generic parameter representing the entity's status field, consider adding a type constraint or helper to avoid the as any:

🔧 Suggested approach

-  async getByStatus(status: Status): Promise<T[]> {
-    return Array.from(this.data.values()).filter(
-      (entity) => (entity as any).status === status,
-    );
-  }
+  async getByStatus(status: Status): Promise<T[]> {
+    return Array.from(this.data.values()).filter(
+      (entity) => 'status' in entity && entity.status === status,
+    );
+  }

Alternatively, extend TrackedActionBase to include status: string if all tracked entities have a status field. As per coding guidelines: "Use proper typing over any or type assertions in TypeScript."

typescript/rebalancer/src/core/RebalancerService.test.ts (1)

185-273: The setupDaemonTest helper is a beast, but a useful one.

This consolidates the complex wiring needed for daemon mode tests. One thing though - line 216 uses a comma operator trick:

...(intentIndex++, {}),

This increments intentIndex as a side effect while spreading an empty object. It works, but it's the kind of clever that might confuse the next person reading it. Consider making the increment explicit:

🔧 More readable alternative

   (actionTracker.createRebalanceIntent as Sinon.SinonStub).callsFake(
-    async () => ({
-      id: options.intentIds?.[intentIndex] ?? `intent-${intentIndex + 1}`,
-      status: 'not_started' as const,
-      ...(intentIndex++, {}),
-    }),
+    async () => {
+      const id = options.intentIds?.[intentIndex] ?? `intent-${intentIndex + 1}`;
+      intentIndex++;
+      return {
+        id,
+        status: 'not_started' as const,
+      };
+    },
   );

typescript/rebalancer/src/strategy/StrategyFactory.ts (1)

113-125: Drop the override type cast; tighten the types instead.
The as ChainMap<Partial<...>> hides mismatches. Better to make StrategyConfig.override already typed as ChainMap<Partial<BridgeConfig>> (or a compatible type) and pass it through without casting.

♻️ Suggested tweak

-        override: config.override as ChainMap<
-          Partial<{ bridge: string; bridgeMinAcceptedAmount: string | number }>
-        >,
+        override: config.override,

As per coding guidelines: "Avoid unnecessary type casts (`as` assertions), especially `as unknown as X` double-casts in TypeScript."

typescript/rebalancer/src/strategy/CollateralDeficitStrategy.ts (1)

296-315: Replace the StrategyRoute cast with a safer shape.
Casting Route to StrategyRoute for bridge sidesteps typing. A Route plus optional bridge keeps it safe without assertions.

♻️ Suggested tweak

-  private filterByConfiguredBridges(pendingRebalances?: Route[]): Route[] {
+  private filterByConfiguredBridges(
+    pendingRebalances?: Array<Route & Partial<Pick<StrategyRoute, 'bridge'>>>,
+  ): Array<Route & Partial<Pick<StrategyRoute, 'bridge'>>> {
...
-    return pendingRebalances.filter((rebalance) => {
-      const bridge = (rebalance as StrategyRoute).bridge;
+    return pendingRebalances.filter((rebalance) => {
+      const bridge = rebalance.bridge;

As per coding guidelines: "Avoid unnecessary type casts (`as` assertions), especially `as unknown as X` double-casts in TypeScript."

typescript/rebalancer/src/config/types.ts (1)

251-256: Avoid type cast for override config access.

Now look here - the coding guidelines say we shouldn't be usin' type assertions like as { bridge?: string }. Instead, we could handle this more properly with the actual schema type or a type guard.

♻️ Suggested refactor

       if (chainConfig.override) {
         for (const overrideConfig of Object.values(chainConfig.override)) {
-          const override = overrideConfig as { bridge?: string };
-          if (override.bridge) {
-            bridges.add(override.bridge);
+          if ('bridge' in overrideConfig && typeof overrideConfig.bridge === 'string') {
+            bridges.add(overrideConfig.bridge);
           }
         }
       }

As per coding guidelines: "Avoid unnecessary type casts (as assertions)... Prefer type guards for runtime narrowing in TypeScript rather than type assertions."

typescript/rebalancer/src/strategy/MinAmountStrategy.ts (1)

156-164: Non-null assertion could mask initialization issues.

That ! on this.tokensByChainName! is a bit risky, like wanderin' into the woods without checkin' for pitchforks first. If tokensByChainName is ever undefined, this'll throw at runtime with a confusing error instead of a helpful one.

Consider whether the base class guarantees this is always set, or add defensive handling.

♻️ Suggested defensive check

 protected getTokenByChainName(chainName: string): Token {
-  const token = this.tokensByChainName![chainName];
+  if (!this.tokensByChainName) {
+    throw new Error('tokensByChainName not initialized');
+  }
+  const token = this.tokensByChainName[chainName];

   if (token === undefined) {
     throw new Error(`Token not found for chain ${chainName}`);
   }

   return token;
 }

typescript/rebalancer/src/monitor/Monitor.ts (1)

41-63: Type cast on reorgPeriod string could be avoided.

At line 49, there's a cast as EthJsonRpcBlockParameterTag. The coding guidelines suggest avoiding unnecessary type casts. Since you're checking typeof reorgPeriod === 'string', TypeScript should narrow the type, but the return type ConfirmedBlockTag might need adjustment.

♻️ Possible approach

If ConfirmedBlockTag already includes EthJsonRpcBlockParameterTag | number | undefined, the cast might be unnecessary. Otherwise, consider whether the type definitions could be aligned to avoid the cast.

       if (typeof reorgPeriod === 'string') {
-        return reorgPeriod as EthJsonRpcBlockParameterTag;
+        // reorgPeriod is already narrowed to string, which should match EthJsonRpcBlockParameterTag
+        return reorgPeriod;
       }

As per coding guidelines: "Avoid unnecessary type casts (as assertions)."

typescript/rebalancer/src/tracking/ActionTracker.ts (1)

518-539: Type cast on reorgPeriod could be avoided.

Similar to Monitor.ts, line 526 has as ConfirmedBlockTag. Consider aligning types to avoid the cast.

♻️ Suggested adjustment

       if (typeof reorgPeriod === 'string') {
-        return reorgPeriod as ConfirmedBlockTag;
+        return reorgPeriod;
       }

If the type narrowing doesn't work due to ConfirmedBlockTag definition, consider adjusting the type definition itself.

As per coding guidelines: "Avoid unnecessary type casts (as assertions)."

typescript/rebalancer/src/strategy/BaseStrategy.ts (1)

471-495: Consider logging when token is missing for minimum amount check.

When tokensByChainName exists but doesn't contain the origin chain's token, the minimum amount check is silently skipped. This might be intentional, but a debug log could help troubleshoot configuration issues.

🔧 Optional: Add debug logging for missing token

       if (this.tokensByChainName) {
         const token = this.tokensByChainName[route.origin];
         if (token) {
           const bridgeConfig = this.getBridgeConfigForRoute(
             route.origin,
             route.destination,
           );
           const minAmount = BigInt(
             toWei(bridgeConfig.bridgeMinAcceptedAmount, token.decimals),
           );
           if (route.amount < minAmount) {
             this.logger.info(
               {
                 context: this.constructor.name,
                 origin: route.origin,
                 destination: route.destination,
                 amount: route.amount.toString(),
                 minAmount: minAmount.toString(),
               },
               'Dropping route below bridgeMinAcceptedAmount',
             );
             return false;
           }
+        } else {
+          this.logger.debug(
+            {
+              context: this.constructor.name,
+              origin: route.origin,
+            },
+            'Token not found for origin chain, skipping minAmount check',
+          );
         }
       }

typescript/rebalancer/src/test/helpers.ts (2)

49-60: Consider deterministic intentId generation for test reproducibility.

Using Date.now() at line 53 could lead to non-deterministic test outputs. For most tests this is fine, but if any test relies on intentId comparison, it might get swampy.

🔧 Optional: Use counter-based ID

+let testRouteCounter = 0;
+
 export function buildTestRebalanceRoute(
   overrides: Partial<RebalanceRoute> = {},
 ): RebalanceRoute {
   return {
-    intentId: overrides.intentId ?? `test-route-${Date.now()}`,
+    intentId: overrides.intentId ?? `test-route-${++testRouteCounter}`,
     origin: 'ethereum',
     destination: 'arbitrum',
     amount: ethers.utils.parseEther('100').toBigInt(),
     bridge: TEST_ADDRESSES.bridge,
     ...overrides,
   };
 }

---

441-448: Double cast as any as RebalancerConfig hides type errors.

This cast at line 448 masks potential type mismatches in the config construction. If RebalancerConfig requires fields not being set here, you won't know until runtime. Like ignoring warning signs in a swamp - might be fine, might not be.

Per coding guidelines, prefer explicit object composition or Partial<> patterns over double casts.

♻️ Consider using Partial type

 export function buildTestConfig(
   overrides: Partial<RebalancerConfig> = {},
   chains: string[] = ['chain1'],
-): RebalancerConfig {
+): Partial<RebalancerConfig> & Pick<RebalancerConfig, 'warpRouteId' | 'strategyConfig'> {
   // ... existing logic ...
   
   return {
     warpRouteId: 'test-route',
     ...restOverrides,
     strategyConfig,
-  } as any as RebalancerConfig;
+  };
 }

Alternatively, ensure all required fields of RebalancerConfig are explicitly set with defaults.

typescript/rebalancer/src/tracking/store/InMemoryStore.ts (1)

31-40: Drop the as T cast in update.
You can keep type safety without the assertion by assigning to a typed variable.

♻️ Proposed tweak

-    this.data.set(id, {
-      ...existing,
-      ...updates,
-      updatedAt: Date.now(),
-    } as T);
+    const updated: T = {
+      ...existing,
+      ...updates,
+      updatedAt: Date.now(),
+    };
+    this.data.set(id, updated);

As per coding guidelines: Avoid unnecessary type casts (as assertions), especially as unknown as X double-casts in TypeScript. If types don't match, fix the underlying types rather than casting.

typescript/rebalancer/src/utils/ExplorerClient.ts (1)

46-64: Avoid any for Explorer rows; use a typed shape.
Keeps the normalization path safer and self-documenting.

♻️ Example direction

-  private normalizeExplorerMessage(msg: any): ExplorerMessage {
+  type ExplorerMessageRow = {
+    msg_id: string;
+    origin_domain_id: number;
+    destination_domain_id: number;
+    sender: string;
+    recipient: string;
+    origin_tx_hash: string;
+    origin_tx_sender: string;
+    origin_tx_recipient: string;
+    is_delivered: boolean;
+    message_body: string;
+  };
+
+  private normalizeExplorerMessage(msg: ExplorerMessageRow): ExplorerMessage {
     ...
   }

-    const validatedRows = rows.filter((msg: any) => {
+    const validatedRows = rows.filter((msg: ExplorerMessageRow) => {
       ...
     });

-    return messages.map((msg: any) => this.normalizeExplorerMessage(msg));
+    return messages.map((msg: ExplorerMessageRow) =>
+      this.normalizeExplorerMessage(msg),
+    );

As per coding guidelines: Use proper typing over any or type assertions in TypeScript.

Also applies to: 155-165, 262-265, 358-360

typescript/rebalancer/src/strategy/CollateralDeficitStrategy.ts (1)

383-389: Non-null assertion might bite ye later.

Line 384 uses this.tokensByChainName! with a non-null assertion. While the constructor passes tokensByChainName to the base class, if BaseStrategy stores it as optional and it somehow ends up undefined, this would throw at runtime without a helpful error message. The check on line 385-387 would never trigger.

Consider a defensive approach or verifying the base class guarantees this field is always set when passed to the constructor.

♻️ Defensive alternative

   protected getTokenByChainName(chainName: string): Token {
-    const token = this.tokensByChainName![chainName];
+    if (!this.tokensByChainName) {
+      throw new Error('tokensByChainName not initialized');
+    }
+    const token = this.tokensByChainName[chainName];
     if (token === undefined) {
       throw new Error(`Token not found for chain ${chainName}`);
     }
     return token;
   }

typescript/rebalancer/src/factories/RebalancerContextFactory.ts (2)

249-254: Multiple tokens per domain would overwrite - likely fine, but heads up.

If warpCore.tokens ever contains multiple tokens for the same chain, the later one overwrites the earlier. For standard warp routes this shouldn't happen, but if it does, you'd have a sneaky bug lurkin' in the swamp.

---

276-284: Redundant logging: routerCount and domainCount are identical.

Both values are Object.keys(routersByDomain).length. Either remove one or rename to somethin' more descriptive if they're meant to be distinct in the future. Not a big deal, just a small duplication.

♻️ Proposed cleanup

     this.logger.debug(
       {
         warpRouteId: this.config.warpRouteId,
         routerCount: Object.keys(routersByDomain).length,
         bridgeCount: bridges.length,
-        domainCount: Object.keys(routersByDomain).length,
       },
       'ActionTracker created successfully',
     );

typescript/rebalancer/src/utils/ExplorerClient.ts (2)

156-165: Inline hex normalization duplicates the helper logic.

There's a perfectly good normalizeHex function inside normalizeExplorerMessage, but here we're doin' the same dance manually. It's like buildin' two outhouses when one would do.

Consider extractin' normalizeHex as a private class method and reusing it here:

♻️ Suggested refactor

+  private normalizeHex(hex: string): string {
+    if (!hex) return hex;
+    return hex.startsWith('\\x') ? '0x' + hex.slice(2) : hex;
+  }
+
   private normalizeExplorerMessage(msg: any): ExplorerMessage {
-    const normalizeHex = (hex: string): string => {
-      if (!hex) return hex;
-      return hex.startsWith('\\x') ? '0x' + hex.slice(2) : hex;
-    };
-
     return {
-      msg_id: normalizeHex(msg.msg_id),
+      msg_id: this.normalizeHex(msg.msg_id),
       // ... rest uses this.normalizeHex
     };
   }

Then in the validation block:

-      const normalizedMsgRouter = msg.origin_tx_recipient?.startsWith('\\x')
-        ? '0x' + msg.origin_tx_recipient.slice(2)
-        : msg.origin_tx_recipient;
+      const normalizedMsgRouter = this.normalizeHex(msg.origin_tx_recipient);

---

126-130: No timeout on fetch requests could cause hangin' issues.

If the Explorer's havin' a bad day and doesn't respond, these fetch calls will just sit there like a donkey waitin' for breakfast. For a daemon that polls regularly, this could gum up the works.

Consider adding an AbortController with a timeout:

♻️ Suggested pattern

const controller = new AbortController();
const timeoutId = setTimeout(() => controller.abort(), 30_000); // 30s timeout

try {
  const res = await fetch(this.baseUrl, {
    method: 'POST',
    headers: { 'Content-Type': 'application/json' },
    body: JSON.stringify({ query, variables }),
    signal: controller.signal,
  });
  // ... handle response
} finally {
  clearTimeout(timeoutId);
}

typescript/rebalancer/src/factories/RebalancerContextFactory.ts (1)

262-271: Comment numbering is off - step 5 got lost somewhere.

The comments go 1, 2, 3, 4... then jump to 6. Someone must've removed step 5 and forgot to renumber. It's a small thing, but it makes the code look like it lost count after a long day in the swamp.

✏️ Suggested fix

-    // 6. Create ActionTracker
+    // 5. Create ActionTracker
     const tracker = new ActionTracker(

And update the following comment:

-    // 7. Create InflightContextAdapter
+    // 6. Create InflightContextAdapter
     const adapter = new InflightContextAdapter(tracker, this.multiProvider);

typescript/rebalancer/src/core/RebalancerService.ts (1)

248-256: Manual rebalance generates an intentId but skips ActionTracker entirely.

The manual path fires up a random UUID for the route, but it never registers that intent with ActionTracker like the daemon mode does. The daemon's executeWithTracking creates intents first, then runs the rebalance—but manual just goes straight to rebalancing without that tracking step.

Since manual rebalances are one-off operations outside the strategy cycle, not tracking them makes sense. But the orphaned intentId means inflight context won't include manually triggered rebalances, which could be a blind spot if someone fires one off while the daemon's running.

Worth adding a comment explaining this is intentional, or at minimum ensuring any monitoring dashboards account for untracked manual operations.

typescript/rebalancer/src/tracking/InflightContextAdapter.test.ts (1)

15-28: The mock setup works, but those as any casts are a wee bit swampy.

Per coding guidelines, type casts should be avoided where possible. For test mocks where you only need specific methods, consider using Partial<> patterns or creating a minimal interface that satisfies the required shape.

// Alternative approach using Pick for type safety
const actionTracker: Pick<IActionTracker, 'getActiveRebalanceIntents' | 'getInProgressTransfers'> = {
  getActiveRebalanceIntents: Sinon.stub(),
  getInProgressTransfers: Sinon.stub(),
};

That said, this is test code and the current approach is functional - just something to keep in mind for clarity.

typescript/rebalancer/src/core/RebalancerService.test.ts (2)

212-218: This increment pattern is a bit like finding a donkey in your swamp - unexpected.

The comma operator (intentIndex++, {}) increments the counter while returning an empty object for the spread. While clever, it's not immediately obvious what's happening. Consider a more explicit approach:

♻️ Suggested refactor for clarity

   (actionTracker.createRebalanceIntent as Sinon.SinonStub).callsFake(
-    async () => ({
-      id: options.intentIds?.[intentIndex] ?? `intent-${intentIndex + 1}`,
-      status: 'not_started' as const,
-      ...(intentIndex++, {}),
-    }),
+    async () => {
+      const id = options.intentIds?.[intentIndex] ?? `intent-${intentIndex + 1}`;
+      intentIndex++;
+      return {
+        id,
+        status: 'not_started' as const,
+      };
+    },
   );

---

52-66: Heavy as unknown as casting in the mock setup.

Per coding guidelines, double-casts like as unknown as MultiProvider should be avoided where possible. For test mocks, consider creating a type-safe mock builder or using Partial<MultiProvider> with the minimum required properties. That said, I get it - mocking complex SDK types is sometimes like draining a swamp.

typescript/rebalancer/src/utils/ExplorerClient.ts (1)

43-64: Define an ExplorerMessageRow type for raw explorer data.

The code uses any in three places when handling raw GraphQL rows: the normalizeExplorerMessage parameter, the filter callback, and the map callbacks. A dedicated type keeps things structured and safer—like layers in an ogre, each layer has its place.

♻️ Proposed refactor

+type ExplorerMessageRow = {
+  msg_id: string | null;
+  origin_domain_id: number;
+  destination_domain_id: number;
+  sender: string | null;
+  recipient: string | null;
+  origin_tx_hash: string | null;
+  origin_tx_sender: string | null;
+  origin_tx_recipient: string | null;
+  is_delivered: boolean;
+  message_body: string | null;
+};
+
-  private normalizeExplorerMessage(msg: any): ExplorerMessage {
+  private normalizeExplorerMessage(msg: ExplorerMessageRow): ExplorerMessage {

-    const validatedRows = rows.filter((msg: any) => {
+    const validatedRows = rows.filter((msg: ExplorerMessageRow) => {

-    return messages.map((msg: any) => this.normalizeExplorerMessage(msg));
+    return messages.map((msg: ExplorerMessageRow) =>
+      this.normalizeExplorerMessage(msg),
+    );

typescript/rebalancer/src/strategy/CollateralDeficitStrategy.test.ts (1)

104-115: Minor documentation nit on units terminology.

The comment says "1005000000 (wei)" but USDC has 6 decimals, not 18 like ETH. Callin' it "wei" is a bit misleadin' - might want to say "base units" or "smallest units" instead to keep things clear for folks who wander into this test file later.

📝 Suggested comment clarification

-      // chain1: deficit = |-5 USDC| + 1000 USDC = 1005 USDC = 1005000000 (wei)
+      // chain1: deficit = |-5 USDC| + 1000 USDC buffer = 1005 USDC = 1_005_000_000 (base units, 6 decimals)

typescript/rebalancer/src/strategy/MinAmountStrategy.test.ts (1)

569-639: Good catch on the zero-amount edge case, this is exactly what me swamp needs.

This test covers an important scenario where deficit scaling can produce zero-amount routes due to integer division. The test setup is solid - it creates conditions where totalSurplus=1 and totalDeficit=198 would scale each deficit to (99 * 1) / 198 = 0.

One small thing though - the test has a redundant assertion. Lines 626-631 and 635-638 both check that all routes have non-zero amounts. The second assertion is sufficient on its own.

🧅 Consider simplifying the assertions

-      // All routes should have non-zero amounts
-      // (Chai's greaterThan doesn't support BigInt, so use direct comparison)
-      for (const route of routes) {
-        expect(
-          route.amount > 0n,
-          `Route amount should be > 0, got ${route.amount}`,
-        ).to.be.true;
-      }
-
-      // The single token of surplus may produce one route (or none if both scale to 0)
-      // Either way, no zero-amount routes should exist
       expect(
         routes.every((r) => r.amount > 0n),
         'All routes must have non-zero amounts',
       ).to.be.true;

typescript/rebalancer/src/strategy/WeightedStrategy.test.ts (1)

423-429: Avoid the double-cast; define a minimal interface instead.

Now, I know it might seem like a quick solution, but this as unknown as Token double-cast goes against the grain of proper typing. Per the coding guidelines, we should fix the underlying types rather than forcing them through a cast like an ogre squeezing through a small doorway.

Consider defining a minimal mock interface that matches only what the tests need:

♻️ Suggested approach

-    function createMockToken(chainName: string, decimals = 18): Token {
-      return {
-        chainName,
-        decimals,
-        addressOrDenom: ethers.constants.AddressZero,
-      } as unknown as Token;
-    }
+    // Define minimal interface matching what WeightedStrategy actually uses
+    interface MockToken {
+      chainName: string;
+      decimals: number;
+      addressOrDenom: string;
+    }
+
+    function createMockToken(chainName: string, decimals = 18): MockToken {
+      return {
+        chainName,
+        decimals,
+        addressOrDenom: ethers.constants.AddressZero,
+      };
+    }

Then update tokensByChainName typing to ChainMap<MockToken> for these tests, or use Partial<Token> if the strategy accepts it.

As per coding guidelines: "Avoid unnecessary type casts (as assertions), especially as unknown as X double-casts in TypeScript."

typescript/rebalancer/src/strategy/StrategyFactory.ts (1)

113-129: Consider consolidating extractBridgeConfigs with the test helper.

There's a similar extractBridgeConfigs function in typescript/rebalancer/src/test/helpers.ts. While the implementations differ slightly (this one handles override field), it might be worth having a single source of truth to avoid the two driftin' apart over time.

Also, the type cast at line 122-125 could potentially be avoided if the StrategyConfig type properly defined the override field structure. Not a blocker, but somethin' to tidy up later.

typescript/rebalancer/src/strategy/MinAmountStrategy.ts (1)

156-164: Non-null assertion could bite you later.

That ! on this.tokensByChainName![chainName] assumes tokensByChainName is always defined. The subsequent check handles the case where the token itself is missing, but if tokensByChainName is undefined, you'll get a runtime error before that check runs.

Consider adding an early guard or making the type guarantee stronger.

💡 Optional defensive check

 protected getTokenByChainName(chainName: string): Token {
+  if (!this.tokensByChainName) {
+    throw new Error('tokensByChainName not initialized');
+  }
-  const token = this.tokensByChainName![chainName];
+  const token = this.tokensByChainName[chainName];

   if (token === undefined) {
     throw new Error(`Token not found for chain ${chainName}`);
   }

   return token;
 }

typescript/rebalancer/src/core/Rebalancer.ts (1)

533-559: Heads up on single Dispatch message assumption.

Line 556 takes dispatchedMessages[0].id without checking if there could be multiple. If a rebalance transaction somehow emits multiple Dispatch events (unlikely but possible with complex bridge interactions), you'd only capture the first one.

Might want to log a warning if dispatchedMessages.length > 1 just so you know when it happens.

💡 Add warning for multiple dispatches

   private buildResult(
     transaction: PreparedTransaction,
     receipt: providers.TransactionReceipt,
   ): RebalanceExecutionResult {
     const { origin, destination } = transaction.route;
     const dispatchedMessages = HyperlaneCore.getDispatchedMessages(receipt);

     if (dispatchedMessages.length === 0) {
       this.logger.error(
         { origin, destination, txHash: receipt.transactionHash },
         'No Dispatch event found in confirmed rebalance receipt',
       );
       return {
         route: transaction.route,
         success: false,
         error: `Transaction confirmed but no Dispatch event found`,
         txHash: receipt.transactionHash,
       };
     }

+    if (dispatchedMessages.length > 1) {
+      this.logger.warn(
+        {
+          origin,
+          destination,
+          txHash: receipt.transactionHash,
+          messageCount: dispatchedMessages.length,
+        },
+        'Multiple Dispatch events found, using first',
+      );
+    }
+
     return {
       route: transaction.route,
       success: true,
       messageId: dispatchedMessages[0].id,
       txHash: receipt.transactionHash,
     };
   }

typescript/rebalancer/src/config/types.ts (1)

251-258: Type cast in getAllBridges could be cleaner.

The as { bridge?: string } cast on line 253 works but isn't ideal per the coding guidelines about avoiding unnecessary type assertions. Since RebalancerBridgeConfigSchema.partial() is the type for override values, you could potentially narrow the type more explicitly.

💡 Type-safe alternative

       if (chainConfig.override) {
         for (const overrideConfig of Object.values(chainConfig.override)) {
-          const override = overrideConfig as { bridge?: string };
-          if (override.bridge) {
-            bridges.add(override.bridge);
+          if ('bridge' in overrideConfig && typeof overrideConfig.bridge === 'string') {
+            bridges.add(overrideConfig.bridge);
           }
         }
       }

Based on learnings: "Avoid unnecessary type casts (as assertions)... Prefer type guards for runtime narrowing in TypeScript rather than type assertions."

typescript/rebalancer/src/monitor/Monitor.ts (1)

41-63: Confirmed block calculation looks reasonable, but the cast could be cleaner.

The logic to compute latestBlock - reorgPeriod is solid. However, line 49's cast as EthJsonRpcBlockParameterTag assumes any string reorgPeriod is a valid block tag (like 'finalized' or 'safe'). If the metadata contains an unexpected string, this could pass through unchecked.

💡 Consider validating string values

       if (typeof reorgPeriod === 'string') {
+        // Validate it's a known block tag
+        const validTags: EthJsonRpcBlockParameterTag[] = ['latest', 'earliest', 'pending', 'safe', 'finalized'];
+        if (!validTags.includes(reorgPeriod as EthJsonRpcBlockParameterTag)) {
+          this.logger.warn(
+            { chain: chainName, reorgPeriod },
+            'Unknown reorgPeriod string, using latest',
+          );
+          return undefined;
+        }
         return reorgPeriod as EthJsonRpcBlockParameterTag;
       }

typescript/rebalancer/src/test/helpers.ts (6)

37-91: Consider a deterministic default intentId in fixtures.

Line 53 uses Date.now() which can make snapshots or golden tests jittery. Maybe default to a stable string/counter and let overrides opt into randomness.

---

95-105: Avoid the as unknown as TokenAmount double-cast.

The double-cast hides mismatched shapes and makes refactors brittle. Prefer constructing a TokenAmount via the SDK factory/constructor or a satisfies TokenAmount mock with the full required shape.

As per coding guidelines: Avoid unnecessary type casts (as assertions), especially as unknown as X double-casts in TypeScript.

---

186-247: Prefer a typed mock over as unknown as MultiProvider.

The double-cast bypasses type checks. Consider defining a dedicated mock type (or using satisfies MultiProvider) so missing methods are caught early.

As per coding guidelines: Use proper typing over any or type assertions in TypeScript.

---

301-321: Type safety slip with protocol: 'ethereum' as any.

If protocol has a union/enum type, prefer the exact value instead of any. This keeps metadata mocks aligned with real config.

As per coding guidelines: Avoid unnecessary type casts (as assertions), especially as unknown as X double-casts in TypeScript.

---

333-357: Avoid token as unknown as Token in the test context.

That cast can mask missing properties on the mock token. Consider shaping createMockToken to satisfy Token directly.

As per coding guidelines: Adjust interface definitions to be compatible rather than using type casts in TypeScript.

---

392-448: Reduce any in buildTestConfig to catch config drift.

The as any and as any as RebalancerConfig make it easy to miss breaking changes in strategy config. A typed builder (or satisfies RebalancerConfig) would give you stronger guarantees.

As per coding guidelines: Use proper typing over any or type assertions in TypeScript.

typescript/cli/src/tests/ethereum/warp/warp-rebalancer.e2e-test.ts (1)

1115-1120: Stale comment - timeout value mismatch.

The comment says "10s" but the actual timeout is 30000ms (30 seconds). Small thing, but comments that lie are like signs pointin' to the wrong swamp.

📝 Suggested fix

-      const timeout = 30000; // 10s
+      const timeout = 30000; // 30s

typescript/rebalancer/src/metrics/Metrics.ts (1)

64-68: Consider handling server close errors.

The server.close() callback receives an optional error parameter that's currently being ignored. During shutdown, this might be fine, but ye might want to at least log it so problems don't get lost in the swamp.

🔧 Optional: Handle close errors

 close(): Promise<void> {
-  return new Promise((resolve) => {
-    this.server.close(() => resolve());
+  return new Promise((resolve, reject) => {
+    this.server.close((err) => {
+      if (err) {
+        reject(err);
+      } else {
+        resolve();
+      }
+    });
   });
 }

typescript/rebalancer/src/core/RebalancerService.ts (1)

511-551: Consider adding defensive check for actionTracker.

The non-null assertions (this.actionTracker!) on lines 518 and 538 are safe in the current flow since executeWithTracking checks for existence. But if this method ever gets called from elsewhere, it could lead to a null pointer situation. A wee guard at the top wouldn't hurt.

🔧 Optional: Add defensive check

 private async processExecutionResults(
   results: RebalanceExecutionResult[],
 ): Promise<void> {
+  if (!this.actionTracker) {
+    this.logger.warn('ActionTracker not available, skipping result processing');
+    return;
+  }
+
   for (const result of results) {
     const intentId = result.route.intentId;

     if (result.success && result.messageId) {
-      await this.actionTracker!.createRebalanceAction({
+      await this.actionTracker.createRebalanceAction({

typescript/rebalancer/src/utils/ExplorerClient.ts (1)

134-148: Consider extracting the duplicated error handling into a helper.

The error extraction logic (try JSON, fall back to text, fall back to message) appears three times. A wee helper method would tidy things up.

♻️ Optional refactor

private async extractErrorDetails(res: Response): Promise<string> {
  try {
    const errorJson = await res.json();
    return JSON.stringify(errorJson);
  } catch (_e) {
    try {
      return await res.text();
    } catch (_textError) {
      return 'Unable to read response body';
    }
  }
}

Then replace each error block with:

if (!res.ok) {
  const errorDetails = await this.extractErrorDetails(res);
  throw new Error(`Explorer query failed: ${res.status} ${errorDetails}`);
}

Also applies to: 247-260, 343-356

typescript/rebalancer/src/test/helpers.ts (1)

95-106: Double cast in test helper.

The as unknown as TokenAmount pattern is used here because the mock only implements a subset of TokenAmount. In test helpers this is generally acceptable, but if you find yourself needing the full type more often, consider creating a proper mock class or using a library like sinon-stub-whole-object.

Based on learnings: Avoid unnecessary type casts (as assertions), especially as unknown as X double-casts in TypeScript.

typescript/rebalancer/src/strategy/WeightedStrategy.test.ts (1)

423-429: Avoid the as unknown as Token double-cast, it's not exactly the fairest approach in the land.

Per coding guidelines, double-casts should be avoided. This mock could use Partial<Token> or a proper mock/stub that satisfies the interface requirements instead of forcing the type through unknown.

🧅 Suggested fix

-    function createMockToken(chainName: string, decimals = 18): Token {
-      return {
-        chainName,
-        decimals,
-        addressOrDenom: ethers.constants.AddressZero,
-      } as unknown as Token;
-    }
+    function createMockToken(chainName: string, decimals = 18): Partial<Token> {
+      return {
+        chainName,
+        decimals,
+        addressOrDenom: ethers.constants.AddressZero,
+      };
+    }

Then update the tokensByChainName type to ChainMap<Partial<Token>> or adjust the strategy constructor to accept Partial<Token> for test scenarios.

As per coding guidelines: "Avoid unnecessary type casts (as assertions), especially as unknown as X double-casts in TypeScript."

typescript/rebalancer/src/factories/RebalancerContextFactory.ts (1)

276-284: Redundant logging fields - routerCount and domainCount are the same value.

Both routerCount and domainCount derive from Object.keys(routersByDomain).length. Consider removing one or renaming for clarity if they're meant to convey different concepts.

🧅 Optional cleanup

     this.logger.debug(
       {
         warpRouteId: this.config.warpRouteId,
-        routerCount: Object.keys(routersByDomain).length,
+        domainCount: Object.keys(routersByDomain).length,
         bridgeCount: bridges.length,
-        domainCount: Object.keys(routersByDomain).length,
       },
       'ActionTracker created successfully',
     );

typescript/rebalancer/src/strategy/StrategyFactory.ts (1)

113-129: The extractBridgeConfigs helper has a type assertion that could be tightened up.

The cast at line 122-124 handles the loose override typing from config. While not a double-cast, consider defining stronger types in the config schema to avoid this assertion, or add a runtime validation if the override structure could vary.

🧅 Potential improvement

If the override field's type in StrategyConfig could be more precisely typed upstream, this cast wouldn't be needed. Alternatively, add a type guard:

private static extractBridgeConfigs(
  strategyConfig: StrategyConfig,
): ChainMap<BridgeConfigWithOverride> {
  const bridgeConfigs: ChainMap<BridgeConfigWithOverride> = {};

  for (const [chain, config] of Object.entries(strategyConfig.chains)) {
    bridgeConfigs[chain] = {
      bridge: config.bridge,
      bridgeMinAcceptedAmount: config.bridgeMinAcceptedAmount ?? 0,
      override: config.override ?? {},
    };
  }

  return bridgeConfigs;
}

This assumes BridgeConfigWithOverride.override can accept {} as a default.

typescript/rebalancer/src/strategy/BaseStrategy.ts (2)

84-100: Consider documenting the expected subclass contract for simulation helpers.

The simulatePendingRebalances and simulateProposedRebalances methods are provided as protected helpers, but subclasses are expected to call them within their getCategorizedBalances implementation. This works fine but the implicit contract might catch someone off guard.

A wee note in the class-level or method docstrings about this pattern would help folks not wander into the swamp unprepared.

---

407-410: Minor type inconsistency between simulation methods and abstract signature.

simulateProposedRebalances takes Route[] but the abstract getCategorizedBalances signature uses StrategyRoute[] for proposedRebalances. While this works (StrategyRoute extends Route), it's a bit like having different size boots - one fits inside the other but it's not quite comfortable.

Consider aligning the types for clarity, or adding a comment explaining the intentional flexibility.